Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for #1873 avoid AttributeError raised ... #2013

Merged
merged 1 commit into from Jul 11, 2022
Merged

Fix for #1873 avoid AttributeError raised ... #2013

merged 1 commit into from Jul 11, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jul 7, 2022

from attempting to access language and datatype attributes of non-Literal objects

Fix for #1873 ...

“Using the SPARQL query engine of rdflib 6.1.1, when an assignment's operand is an error (e.g., running a function on an unbound variable), an error is thrown, for example: # AttributeError: 'NotBoundError' object has no attribute 'datatype'. However, the SPARQL 1.1 specification expects that the assigned variable would remain unbound in such case:”

Summary of changes

Added isinstance check to avoid AttributeErrors being raised by attempting to access language and datatypeattribs of non-Literal objects being passed back up the call stack resulting from SPARQL BIND failures/errors.

Checklist

  • Checked that there aren't other open pull requests for the same change.
  • Added tests for any changes that have a runtime impact.
  • Checked that all tests and type checking passes.
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

Avoid raising unrelated AttributeError ...
8d468dd 
from attempting to access language and datatype attributes of non-Literal objects
@ghost ghost changed the title Avoid raising unrelated AttributeError ... Fix for 1873 AttributeError ... Jul 7, 2022
@ghost ghost changed the title Fix for 1873 AttributeError ... Fix for #1873 AttributeError ... Jul 7, 2022
@ghost ghost changed the title Fix for #1873 AttributeError ... Fix for #1873 avoid AttributeError raised ... Jul 7, 2022
@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.224% when pulling 8d468dd on gjhiggins:fix-issue1873-sparql-bind-exception into 8ccebc8 on RDFLib:master.

@ghost ghost self-requested a review July 7, 2022 18:11
@aucampia
Copy link
Member

Just a note for posterity's sake, this patch works because it prevents an AttributeError from occuring, and instead a SPARQLError occurs later, specifically here:

rdflib/rdflib/plugins/sparql/operators.py

Line 307 in 8d468dd

return Literal("".join(string(x) for x in expr.arg), datatype=dt, lang=lang)

The SPARQLError that occurs later is correctly handled as per the standard here:

rdflib/rdflib/plugins/sparql/evaluate.py

Lines 95 to 109 in 8d468dd

def evalExtend(
ctx: QueryContext, extend: CompValue
) -> Generator[FrozenBindings, None, None]:
# TODO: Deal with dict returned from evalPart from GROUP BY
for c in evalPart(ctx, extend.p):
try:
e = _eval(extend.expr, c.forget(ctx, _except=extend._vars))
if isinstance(e, SPARQLError):
raise e
yield c.merge({extend.var: e})
except SPARQLError:
yield c

@aucampia
Copy link
Member

aucampia commented Jul 11, 2022 •
edited
Loading

Will merge this with only one review by tonight if there is no further feedback.

niklasl
niklasl approved these changes Jul 11, 2022
View reviewed changes
Copy link
Member

@niklasl niklasl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@aucampia aucampia merged commit 7aba79a into RDFLib:master Jul 11, 2022
@ghost ghost deleted the fix-issue1873-sparql-bind-exception branch July 17, 2022 22:43
@aucampia aucampia mentioned this pull request Jul 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@coveralls @aucampia @niklasl